Use PMCD returned by mappinganalysis to build minimal graph for query #3488

YannanGao-gs · 2024-08-30T21:14:11Z

Summary

Use PMCD returned by mappinganalysis to build minimal graph for query

clean/update: #2523

How did you test this change?

Test(s) added
Manual testing (please provide screenshots/recordings)
No testing (please provide an explanation)

Screen.Recording.2024-09-26.at.10.28.48.PM.mov

enable switch to load full graph

Screen.Recording.2024-09-26.at.10.27.01.PM.mov

changeset-bot · 2024-08-30T21:14:15Z

🦋 Changeset detected

Latest commit: b505283

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 29 packages

Name	Type
@finos/legend-extension-dsl-data-space	Minor
@finos/legend-application-query	Minor
@finos/legend-extension-dsl-data-space-studio	Patch
@finos/legend-application-studio	Patch
@finos/legend-query-builder	Patch
@finos/legend-server-depot	Patch
@finos/legend-graph	Patch
@finos/legend-application-query-bootstrap	Patch
@finos/legend-application-studio-bootstrap	Patch
@finos/legend-extension-dsl-data-quality	Patch
@finos/legend-extension-dsl-service	Patch
@finos/legend-extension-assortment	Patch
@finos/legend-extension-dsl-diagram	Patch
@finos/legend-extension-dsl-persistence	Patch
@finos/legend-extension-dsl-text	Patch
@finos/legend-extension-store-flat-data	Patch
@finos/legend-extension-store-relational	Patch
@finos/legend-extension-store-service-store	Patch
@finos/legend-vscode-extension-dependencies	Patch
@finos/legend-application-pure-ide	Patch
@finos/legend-application-repl	Patch
@finos/legend-code-editor	Patch
@finos/legend-data-cube	Patch
@finos/legend-lego	Patch
@finos/legend-application-query-deployment	Patch
@finos/legend-application-studio-deployment	Patch
@finos/legend-application-pure-ide-deployment	Patch
@finos/legend-application-repl-deployment	Patch
@finos/legend-server-showcase-deployment	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2024-08-30T21:18:28Z

Codecov Report

Attention: Patch coverage is 17.78976% with 1220 lines in your changes missing coverage. Please review.

Project coverage is 45.37%. Comparing base (595e28d) to head (b505283).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...e/v1/V1_DSL_DataSpace_PureGraphManagerExtension.ts	4.76%	300 Missing ⚠️
...c/components/Core_LegendQueryApplicationPlugin.tsx	14.59%	240 Missing ⚠️
...s/data-space/DataSpaceTemplateQueryCreatorStore.ts	8.50%	183 Missing ⚠️
...rc/stores/data-space/DataSpaceQueryCreatorStore.ts	5.83%	129 Missing ⚠️
...d-application-query/src/stores/QueryEditorStore.ts	23.03%	127 Missing ⚠️
...components/query-builder/DataSpaceQueryBuilder.tsx	0.00%	40 Missing ⚠️
...stores/query-builder/DataSpaceQueryBuilderState.ts	0.00%	34 Missing ⚠️
.../QueryBuilder_LegendApplicationPlugin_Extension.ts	0.00%	28 Missing ⚠️
...r/action/analytics/MappingModelCoverageAnalysis.ts	10.34%	26 Missing ⚠️
...r/src/stores/workflows/ServiceQueryBuilderState.ts	0.00%	19 Missing ⚠️
... and 19 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3488      +/-   ##
==========================================
- Coverage   45.48%   45.37%   -0.11%     
==========================================
  Files        2168     2168              
  Lines      374901   376075    +1174     
  Branches    16060    10807    -5253     
==========================================
+ Hits       170527   170654     +127     
- Misses     203683   204730    +1047     
  Partials      691      691

Files with missing lines	Coverage Δ
...src/__lib__/DSL_DataSpace_LegendQueryNavigation.ts	`59.84% <100.00%> (+0.64%)`	⬆️
...ry/src/application/LegendQueryApplicationConfig.ts	`98.41% <100.00%> (+0.02%)`	⬆️
...s/__test-utils__/QueryEditorComponentTestUtils.tsx	`100.00% <100.00%> (ø)`
...-query/src/stores/LegendQueryApplicationPlugin.tsx	`100.00% <100.00%> (ø)`
...r-group/mapping-editor/MappingExecutionBuilder.tsx	`58.41% <100.00%> (ø)`
...ng-editor/legacy/DEPRECATED__MappingTestEditor.tsx	`69.47% <100.00%> (ø)`
...itor/editor-group/uml-editor/ClassQueryBuilder.tsx	`42.11% <100.00%> (ø)`
...dio/src/stores/editor/EmbeddedQueryBuilderState.ts	`79.66% <100.00%> (+0.17%)`	⬆️
...ol/pure/DSL_DataSpace_PureGraphManagerExtension.ts	`92.13% <100.00%> (+1.85%)`	⬆️
...gend-graph/src/graph-manager/action/query/Query.ts	`40.96% <100.00%> (ø)`
... and 30 more

... and 34 files with indirect coverage changes

packages/legend-application-query/src/components/Core_LegendQueryApplicationPlugin.tsx

YannanGao-gs · 2024-09-04T13:48:04Z

HEADS-UP:

To fix (regressions)

about dataspace will not work due to missing connection (modify result json to have connection path)
dataspace template query is not working due to empty executables
either add diagram analyze coverage in engine or bypass diagram check in secondpass in studio

packages/legend-application-query/src/stores/data-space/DataSpaceQueryCreatorStore.ts

packages/legend-query-builder/src/stores/workflows/ServiceQueryBuilderState.ts

...-data-space/src/graph-manager/protocol/pure/v1/V1_DSL_DataSpace_PureGraphManagerExtension.ts

packages/legend-extension-dsl-data-space/src/stores/query-builder/DataSpaceQueryBuilderState.ts

packages/legend-application-query/src/stores/QueryEditorStore.ts

packages/legend-graph/src/graph-manager/protocol/pure/v1/V1_PureGraphManager.ts

packages/legend-extension-dsl-data-space/src/stores/query-builder/DataSpaceQueryBuilderState.ts

...-data-space/src/graph-manager/protocol/pure/v1/V1_DSL_DataSpace_PureGraphManagerExtension.ts

packages/legend-application-query/src/components/Core_LegendQueryApplicationPlugin.tsx

...-data-space/src/graph-manager/protocol/pure/v1/V1_DSL_DataSpace_PureGraphManagerExtension.ts

packages/legend-graph/src/graph/helpers/DomainHelper.ts

packages/legend-graph/src/graph/helpers/PureLanguageHelper.ts

packages/legend-graph/src/graph-manager/action/analytics/MappingModelCoverageAnalysis.ts

MauricioUyaguari · 2024-10-01T20:00:02Z

packages/legend-graph/src/graph-manager/action/query/Query.ts

-  mapping!: PackageableElementReference<Mapping>;
-  runtime!: PackageableElementReference<PackageableRuntime>;
+  // NOTE: Query can be built before the actual graph is built so we can't have the reference of metamodels here
+  mapping!: string;


why do we need to make this change, shouldnt the graph already be build with a stub mapping and runtime for this not to have to change ?

setUpEdtiorState() has been updated to fetch query instead of LightQuery anymore
https://github.com/finos/legend-studio/pull/3488/files#diff-5dae4710ada9d402191df7256d5cc7d4502e3b40a8f1b9acf960143f7ee6adfeR1293

Therefore, it needs to be changed to use string

what is the reasoning for this change.
We have 3 models for Stored Query.
Query, Light Query and Query Info.
The Query one should be the compiled resolved one if we ever need it. the other two are unresolved ones using string paths etc.
So we should not be changing Query to strings.
Depends on what exactly you need in scope.
Maybe you just need QueryInfo ?

...-data-space/src/graph-manager/protocol/pure/v1/V1_DSL_DataSpace_PureGraphManagerExtension.ts

MauricioUyaguari · 2024-10-01T21:03:52Z

...ension-dsl-data-space/src/__lib__/to-delete/DSL_DataSpace_LegendQueryNavigation_to_delete.ts

@@ -39,13 +39,14 @@ export enum DATA_SPACE_TEMPLATE_QUERY_CREATOR_ROUTE_PATTERN_TOKEN {
  GAV = 'gav',
  DATA_SPACE_PATH = 'dataSpacePath',
  TEMPLATE = 'template',
+  EXECUTION_CONTEXT = 'executionContext',


dont think we need this.

packages/legend-query-builder/src/components/explorer/QueryBuilderFunctionsExplorerPanel.tsx

packages/legend-query-builder/src/components/fetch-structure/QueryBuilderTDSPanel.tsx

packages/legend-graph/src/graph/helpers/PureLanguageHelper.ts

packages/legend-query-builder/src/components/fetch-structure/QueryBuilderTDSPanel.tsx

MauricioUyaguari · 2024-10-02T15:04:46Z

packages/legend-application-query/src/__lib__/DSL_DataSpace_LegendQueryNavigation.ts

@@ -39,6 +39,7 @@ export enum DATA_SPACE_TEMPLATE_QUERY_CREATOR_ROUTE_PATTERN_TOKEN {
  GAV = 'gav',
  DATA_SPACE_PATH = 'dataSpacePath',
  TEMPLATE = 'template',
+  EXECUTION_CONTEXT = 'executionContext',


dont think this is needed

This is needed as when trying to build minimal graph, mapping and pmcd are fetched from corresponding sexecutionContext key

Still don't understand. DataspaceInfo should have the template info( which we get from the template id) which has the executon context we need ? right

This is the entry point of building DataspaceAnalyticsResult and light graph is built when building DataspaceAnalyticsResult.
Based on current implementation, template info is built at the last step

packages/legend-application-query/src/stores/data-space/DataSpaceTemplateQueryCreatorStore.ts

MauricioUyaguari · 2024-10-02T15:18:18Z

packages/legend-application-query/src/stores/LegendQueryApplicationPlugin.tsx

@@ -63,6 +65,10 @@ export type NewQueryNavigationPath = (
  editorStore: ExistingQueryEditorStore,
 ) => string | undefined;

+export type QueryGraphBuilderGetter = (


where is this used ?

This is used in https://github.com/finos/legend-studio/pull/3488/files#diff-5dae4710ada9d402191df7256d5cc7d4502e3b40a8f1b9acf960143f7ee6adfeR690
This is for loading an empty function to aovid buildFullGraph for ExistingQueryEditorStore with dataspace context only

packages/legend-application-query/src/components/QueryEditor.tsx

packages/legend-application-query/src/components/data-space/DataSpaceInfo.tsx

MauricioUyaguari

we need a better strategy for the things we have started caching that just a bunch of ifs and checking for info. I think we should change the source of truth for UI to be dataspace info on some of the dataspace specific feature. Im worried about the cost to maintain a lot of this code or even to test it is great.

I think how we built the dataspace info will be determined by whether we get the info from the full graph or the cache dataspace analysis result.
We may want to have a method to determine that this dataspace analysis result is expected to have all this new info (executable and template stuff), maybe adding a version on analysis result and saying if it has version x then we know it has all these fields are expected. Or maybe if the minimal graph is there then we know that it is expected to have this new info. If it has all this new info then we use the cached datasapce info to built it, if not we use the full graph to build this dataspace info.
From the dataspace info is all we need to render these helper forms.

packages/legend-application-query/src/stores/QueryEditorStore.ts

MauricioUyaguari · 2024-10-07T14:26:51Z

packages/legend-graph/src/graph-manager/action/query/Query.ts

-  mapping!: PackageableElementReference<Mapping>;
-  runtime!: PackageableElementReference<PackageableRuntime>;
+  // NOTE: Query can be built before the actual graph is built so we can't have the reference of metamodels here
+  mapping!: string;


what is the reasoning for this change.
We have 3 models for Stored Query.
Query, Light Query and Query Info.
The Query one should be the compiled resolved one if we ever need it. the other two are unresolved ones using string paths etc.
So we should not be changing Query to strings.
Depends on what exactly you need in scope.
Maybe you just need QueryInfo ?

MauricioUyaguari · 2024-10-07T14:47:57Z

packages/legend-query-builder/src/stores/QueryBuilderState.ts

    return queryExeContext;
  }

+  async propagateExecutionContextChange(


[can help] I don't think the extension (getExtraQueryBuilderPropagateExecutionContextChangeHelper) makes much sense. Looking at the implementation you are using only the flags of QueryEditorStore. Maybe we can add include if we use the minimal graph as an input to DataSpaceQueryBuilderState

We should be able to use the input to implement this in DataSpaceQueryBuilderState. We have
onExecutionContextChange
and should have the flags needed in scope

packages/legend-application-query/src/application/LegendQueryApplicationConfig.ts

MauricioUyaguari · 2024-10-07T15:13:17Z

packages/legend-application-query/src/components/Core_LegendQueryApplicationPlugin.tsx

+                supportBuildMinimalGraph
+              ) {
+                try {
+                  const stopWatch = new StopWatch();


additionally below code is repeated.
We should have a buildDataspaceMinialGraph in V1_DSL_DataSpace_PureGraphManagerExtension that takes in minimalEntities, dataspaceInfo, executionKeyToBuild,
and builds the required graph. I think we already have this logic in V1_DSL_DataSpace_PureGraphManagerExtension

MauricioUyaguari · 2024-10-07T15:15:01Z

packages/legend-application-query/src/components/QueryEditor.tsx

@@ -603,6 +603,12 @@ export const QueryEditor = observer(() => {
      !engineConfig.useClientRequestPayloadCompression,
    );

+  const toggleEnableMinialGraphForDataSpaceLoadingPerformance = (): void => {


doesnt this change the local storage value ?

MauricioUyaguari · 2024-10-07T15:19:34Z

the changes to the editors to take info look good (took a quick look) but we need to add tests for these rendering components. do we have any at the moment, didnt see we change any so i assume we don't ?

...ges/legend-extension-dsl-data-space/src/components/DSL_DataSpace_LegendApplicationPlugin.tsx

YannanGao-gs requested a review from a team as a code owner August 30, 2024 21:14

YannanGao-gs marked this pull request as draft August 30, 2024 21:14

YannanGao-gs self-assigned this Aug 30, 2024

YannanGao-gs force-pushed the performanceEnhance branch 3 times, most recently from 2375853 to b8520b0 Compare September 3, 2024 20:27

YannanGao-gs marked this pull request as ready for review September 3, 2024 20:27

YannanGao-gs mentioned this pull request Sep 3, 2024

Use PMCD returned by mappinganalysis to build minimal graph for query #2523

Closed

3 tasks

YannanGao-gs marked this pull request as draft September 3, 2024 20:29

YannanGao-gs force-pushed the performanceEnhance branch 3 times, most recently from aab4beb to e84388e Compare September 4, 2024 13:26